Skip to content

zlib: avoid waiting for paused ZIP iterators - #65278

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:zlib-zipfile-close-hang
Aug 30, 2026
Merged

zlib: avoid waiting for paused ZIP iterators#65278
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
trivikr:zlib-zipfile-close-hang

Conversation

@trivikr

@trivikr trivikr commented Aug 14, 2026

Copy link
Copy Markdown
Member

Fixes: #65277

Track file-backed contentIterator() reads only while I/O is active. This lets ZipFile.close() finish when an iterator is paused after yielding a chunk, while still waiting for reads in flight.


Assisted-by: codex:gpt-5.6-sol

Track file-backed contentIterator() reads only while I/O is active.
This lets ZipFile.close() finish when an iterator is paused after
yielding a chunk, while still waiting for reads in flight.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 14, 2026
@nodejs-github-bot

This comment was marked as resolved.

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (91a99c5) to head (90480cd).
⚠️ Report is 275 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65278      +/-   ##
==========================================
- Coverage   90.32%   90.32%   -0.01%     
==========================================
  Files         751      751              
  Lines      250000   250007       +7     
  Branches    47231    47231              
==========================================
+ Hits       225816   225821       +5     
+ Misses      15566    15565       -1     
- Partials     8618     8621       +3     
Files with missing lines Coverage Δ
lib/internal/zip/entry.js 98.13% <100.00%> (+0.01%) ⬆️

... and 26 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trivikr trivikr added the zlib Issues and PRs related to the zlib module and its compression dependencies. label Aug 14, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a node:zlib ZipFile lifecycle bug where ZipFile.close() could hang if a file-backed ZipEntry.contentIterator() was paused after yielding a chunk, by only counting active I/O as “in-flight” work that close() must wait for.

Changes:

  • Update file-backed ZIP entry streaming (ZipEntry.#rawChunks()) to decrement the shared handle’s reads counter before each yield, and re-increment only when resuming to perform the next read.
  • Add a regression test covering close() resolving while a contentIterator() is paused, and ensuring further iteration fails with ERR_INVALID_STATE.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/parallel/test-zlib-zip-file-lifecycle.js Adds regression coverage for ZipFile.close() behavior with a paused contentIterator().
lib/internal/zip/entry.js Refines in-flight read tracking for file-backed iterator reads to avoid counting time spent paused at yield.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/parallel/test-zlib-zip-file-lifecycle.js
@trivikr
trivikr requested a review from jasnell August 20, 2026 00:34
@trivikr trivikr added review wanted PRs that need review. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 23, 2026
@trivikr
trivikr requested a review from mcollina August 25, 2026 04:55
@nodejs-github-bot

This comment was marked as outdated.

@panva panva removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2026
@trivikr trivikr added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Aug 29, 2026
@nodejs-github-bot

This comment was marked as outdated.

@trivikr trivikr removed the review wanted PRs that need review. label Aug 29, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikr trivikr added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 30, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 884f9cd into nodejs:main Aug 30, 2026
90 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 884f9cd

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zlib: ZipFile.close() hangs after partially consuming file-backed contentIterator()

5 participants